Skip to content

Describe which commit type to use for type annotations#735

Open
voetberg wants to merge 1 commit intorucio:mainfrom
voetberg:conv-commits-type-annotations
Open

Describe which commit type to use for type annotations#735
voetberg wants to merge 1 commit intorucio:mainfrom
voetberg:conv-commits-type-annotations

Conversation

@voetberg
Copy link
Contributor

@voetberg voetberg commented Jan 22, 2026

See this thread for context (mattermost login required)

rdimaio
rdimaio previously approved these changes Jan 24, 2026
View reviewed changes
0xquark
0xquark requested changes Jan 24, 2026
View reviewed changes
Copy link
Member

@0xquark 0xquark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest adding this as an example in the list of examples for choosing the correct type:

documentation/docs/contributing.md

Lines 235 to 258 in ffa2d6d

**Choosing the Right Type:**
Sometimes it's unclear which type to use. Consider the **intent** and **impact** of your change:
| Ambiguity | Choose | When |
|-----------|--------|------|
| `style` vs `refactor` | `style` | Formatting, whitespace, comment tweaks |
| `style` vs `refactor` | `refactor` | Modernizing syntax, restructuring code |
| `fix` vs `refactor` | `fix` | Correcting incorrect behavior |
| `fix` vs `refactor` | `refactor` | Improving code without fixing a bug |
| `docs` vs `style` | `docs` | Updating documentation files or docstrings |
| `docs` vs `style` | `style` | Minor comment formatting within code |
| `test` vs `fix` | `fix` | Fixing flaky test due to race condition |
| `test` vs `fix` | `test` | Adding new test cases or correcting test logic |
**Examples:**
| Scenario | Preferred | Rationale |
|----------|-----------|-----------|
| Replacing `Union[X, None]` with `Optional[X]` across 60+ files | `refactor(Core): Remove deprecated constructs from the typing module` | Modernizes codebase to newer Python conventions |
| Adding type hints to function signatures | `style(Core): Add type hints to transfer functions` | Improves code documentation without changing behavior |
| Fixing a test that fails intermittently due to timing | `fix(Testing): Resolve judge evaluator test flakiness` | Corrects broken behavior in test suite |
| Adding new test cases for edge cases | `test(Testing): Add tests for attaching nonexistent DIDs` | Extends test coverage |
| Updating README with new installation steps | `docs(Documentation): Update installation instructions` | Documentation-only change |

@voetberg voetberg force-pushed the conv-commits-type-annotations branch from ffa2d6d to 005fdca Compare January 28, 2026 23:47
@voetberg
Copy link
Contributor Author

voetberg commented Jan 28, 2026

I would suggest adding this as an example in the list of examples for choosing the correct type:

Ah, never saw that in that table. Corrected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0xquark 0xquark 0xquark requested changes

@rdimaio rdimaio rdimaio approved these changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@voetberg @rdimaio @0xquark